Skip to content

Conversation

@majst01
Copy link
Contributor

@majst01 majst01 commented Nov 19, 2024

This introduces CI on Github Actions which executes make test on every PR. This is required to see the results of the integration tests of #511 and following PRs.

@gabor-boros this would be nice to have first.

@gabor-boros
Copy link
Member

I know it has been a long time, but didn't forget about it. I'll be able to take things from mid January, just to set expectations.

@majst01
Copy link
Contributor Author

majst01 commented Jan 8, 2025

I know it has been a long time, but didn't forget about it. I'll be able to take things from mid January, just to set expectations.

No worries, i am also busy right now

Copy link
Member

@gabor-boros gabor-boros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@majst01 better later than never. I left a few comments

Comment on lines +33 to +36
# - name: Lint
# uses: golangci/golangci-lint-action@v6
# with:
# args: --build-tags integration -p bugs -p unused -D protogetter --timeout=5m
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a golangci-lint config and enable this CI step

Comment on lines +41 to +42


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Comment on lines +43 to +56
# integration:
# runs-on: ubuntu-latest
# steps:
# - name: Checkout
# uses: actions/checkout@v4

# - name: Setup Go
# uses: actions/setup-go@v5
# with:
# go-version-file: 'go.mod'

# - name: Run integration tests
# run: |
# make integration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this disabled because we need RethinkDB running? If yes, you may want to check https://github.com/rethinkdb/rethinkdb-python/blob/gabor/python3-v2.5.0/.github/workflows/build.yml#L27-L42 We could follow the same practice here.


jobs:
test:
name: Docker Build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not building docker

Comment on lines +14 to +16
env:
REGISTRY: ghcr.io
IMAGE_NAME: ${{ github.repository }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seems to be unnecessary

@@ -0,0 +1,56 @@
---
name: Test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: Test
name: Build and Test

@@ -0,0 +1,15 @@
---
name: Release Drafter Action
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: Release Drafter Action
name: Release Draft

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I believe we shouldn't do draft releases without waiting for the test pipeline.

$CHANGES
categories:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As many use conventional commits we may want to follow those labels as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants